feat: enhance query and trace commands with default expressions and d…#21
Conversation
…uration tracking - Updated query commands to use a default expression when omitted, improving usability. - Added duration tracking for run parameters, including status and source information. - Enhanced output formats to include duration in rich, plain, and JSON representations. - Modified help documentation to reflect changes in command usage and default behaviors. - Expanded tests to ensure correct functionality of new features and maintain existing behavior.
There was a problem hiding this comment.
Code Review
This pull request implements a default query expression (run.hash != '') when the query-language expression is omitted in aimx query and trace commands. It also enhances query params by extracting and displaying run duration in rich, plain, and JSON outputs. The review feedback identifies a potential bug in extract_run_duration where a run with a valid duration but missing end_time could be incorrectly marked as "running", and provides a solid code suggestion to check the duration attribute first.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| def extract_run_duration(run: Any) -> RunDuration: | ||
| end_time = _numeric_seconds(getattr(run, "end_time", None)) | ||
| creation_time = _numeric_seconds(getattr(run, "creation_time", None)) | ||
| if creation_time is not None and end_time is None: | ||
| return RunDuration(status="running") | ||
|
|
||
| duration = _numeric_seconds(getattr(run, "duration", None)) | ||
| if duration is not None: | ||
| return RunDuration(seconds=duration, status="available", source="duration") |
There was a problem hiding this comment.
If end_time is unparseable or missing for a completed run, but a valid duration attribute is present, the current logic will prematurely classify the run as "running" (because end_time parses to None while creation_time is valid).
Since duration is the primary and most direct source of truth, checking it first before the running-state fallback ensures that we correctly report the duration and mark the status as "available" even if end_time metadata is corrupted or unparseable.
| def extract_run_duration(run: Any) -> RunDuration: | |
| end_time = _numeric_seconds(getattr(run, "end_time", None)) | |
| creation_time = _numeric_seconds(getattr(run, "creation_time", None)) | |
| if creation_time is not None and end_time is None: | |
| return RunDuration(status="running") | |
| duration = _numeric_seconds(getattr(run, "duration", None)) | |
| if duration is not None: | |
| return RunDuration(seconds=duration, status="available", source="duration") | |
| def extract_run_duration(run: Any) -> RunDuration: | |
| duration = _numeric_seconds(getattr(run, "duration", None)) | |
| if duration is not None: | |
| return RunDuration(seconds=duration, status="available", source="duration") | |
| end_time = _numeric_seconds(getattr(run, "end_time", None)) | |
| creation_time = _numeric_seconds(getattr(run, "creation_time", None)) | |
| if creation_time is not None and end_time is None: | |
| return RunDuration(status="running") |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 88597bd8d2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if creation_time is not None and end_time is None: | ||
| return RunDuration(status="running") |
There was a problem hiding this comment.
Check duration before running fallback
When an Aim run/adapter exposes a valid duration directly but has no readable end_time while creation_time exists, this early return marks the run as running and serializes seconds: null, so query params drops the runtime that the new duration contract says should take precedence. Read and validate duration before treating the missing end time as the running-status fallback.
Useful? React with 👍 / 👎.
…uration tracking